Add absolute links support #1802

joaofreires · 2022-05-07T19:34:14Z

Prepend the `site-url` in links with absolute path.

Problem:

If we host in the book in a sub page we lost the references for the relative root. We already have the site-url in configs, but we do not use it. If we want to link a page in the root of the book, we need to prepend a lot of ../ to reach the actual root, and if we move pages these relative refs could lead us to dead links.

Proposal

Add a new flag use-site-url-as-root in the HtmlConfig.
The changes should have backward compatibility, we'll need to enable the use-site-url-as-root flag in [output.html] section inside the book.toml file to use the new feature.

TODO:

check if other type of links were affected by the changes

Closes: #1764

watsom27 · 2022-09-06T20:11:36Z

Hey,

Is there any update on this one? This fixes the exact issue im having. Would be awesome if we could get this merged/looked at again.

Thanks

joaofreires · 2022-09-06T20:53:31Z

I can fix the merge conflict. Though, I still need someone to review/merge it.

watsom27 · 2022-09-07T16:43:20Z

You'd be an actual hero if you did, thank you.

I replied on the issue thread too so hopefully someone notices.

schungx · 2022-09-08T00:15:08Z

You'd be an actual hero if you did, thank you.

Yes, this is long overdue. I currently have to resort to a templating engine to handle absolute paths. I use absolute paths almost exclusively as they don't require massive editing when I move a chapter up/down folder levels.

watsom27 · 2022-09-29T10:20:26Z

Bump @ehuss

ehuss

Thanks for the PR!

I think it is a little unfortunate to need to add a new config setting for this, but I don't see much of a way around it.

This will need some tests to be added.

It looks like links on the print page aren't using the adjusted links.

It also looks like there are some merge conflicts.

ehuss · 2022-10-14T00:27:17Z

src/utils/mod.rs

+pub fn render_markdown_with_path(
+    text: &str,
+    curly_quotes: bool,
+    path: Option<&Path>,
+    abs_url: Option<&String>,
+) -> String {


Since this is a public API, it can't be changed without a semver break. I think as a workaround, there will need to be another function added which takes the new parameter, and render_markdown_with_path can call it with None. We can then drop the new (or old) function in the future.

Done, I also left a comment to merge those functions in the future.

ehuss · 2022-10-14T00:32:34Z

src/utils/mod.rs

+                }
+                .into();


I'm not sure what the .into() is here for, but I don't think it is necessary?

Suggested change

}

.into();

};

ehuss · 2022-10-14T00:35:47Z

src/utils/mod.rs

+                fixed_link = match abs_url {
+                    Some(abs_url) => format!("{}{}", abs_url.trim_end_matches('/'), &fixed_link),
+                    None => fixed_link,
+                }


Just a minor opinion on style, I think it might be a little clearer to use an if let instead of a match. It is 1 less line and makes it a little clearer that there is "no change" when the value is None.

Suggested change

fixed_link = match abs_url {

Some(abs_url) => format!("{}{}", abs_url.trim_end_matches('/'), &fixed_link),

None => fixed_link,

}

if let Some(abs_url) = abs_url {

fixed_link = format!("{}{}", abs_url.trim_end_matches('/'), &fixed_link);

}

ehuss · 2022-10-14T00:48:30Z

src/config.rs

+    /// Prepend the `site_url` in links with absolute path.
+    pub use_site_url_as_root: bool,


Hm, so technically adding this is a breaking change since HtmlConfig is in the public API. However, we have been adding new fields to this struct for a while now, and nobody has complained. That is something we should definitely fix in the future, but for now I guess we can let it slide. 😦

ehuss · 2022-10-14T00:55:48Z

guide/src/format/configuration/renderers.md

@@ -159,6 +159,7 @@ The following configuration options are available:
  navigation links and script/css imports in the 404 file work correctly, even when accessing
  urls in subdirectories. Defaults to `/`. If `site-url` is set,
  make sure to use document relative links for your assets, meaning they should not start with `/`.
+- **use-site-url-as-root:** Prepend the `site_url` in links with absolute path.


Can you also add this to the TOML summary up above?

links with absolute path

As a user, I would wonder "what links are those"? It could be

links generated by templates, such as those referencing style sheets and JavaScript code

links generated between pages of the book

links I insert using Markdown syntax

joaofreires · 2022-11-13T18:24:56Z

src/utils/mod.rs

+                fixed_link.push_str(&caps["link"].trim_start_matches('/'));
                fixed_link.push_str(".html");
                if let Some(anchor) = caps.name("anchor") {
                    fixed_link.push_str(anchor.as_str());
                }
+            } else if !fixed_link.is_empty() {
+                // prevent links with double slashes
+                fixed_link.push_str(&dest.trim_start_matches('/'));
            } else {
                fixed_link.push_str(&dest);
            };
-            return CowStr::from(fixed_link);
+            if dest.starts_with('/') || path.is_some() {
+                if let Some(abs_url) = abs_url {
+                    fixed_link = format!(
+                        "{}/{}",
+                        abs_url.trim_end_matches('/'),
+                        &fixed_link.trim_start_matches('/')
+                    );
+                }
+            }
+            return CowStr::from(format!("{}", fixed_link));


I needed to add changes to this function because of leading double slashes when the path and abs_url var are defined. I've added some tests to make sure it only affects those cases.

mgeisler · 2023-02-08T07:40:31Z

src/utils/mod.rs

+    text: &str,
+    curly_quotes: bool,
+    path: Option<&Path>,
+    abs_url: Option<&String>,


The &String type should normally not be used — &str is more flexible for the callers since they can pass in both string literals (that would simplify the tests below) and borrowed owned strings.

mgeisler · 2023-02-08T07:46:46Z

I think it is a little unfortunate to need to add a new config setting for this, but I don't see much of a way around it.

Would it not be better to add a syntax to let links use the path_to_root setting instead? That would be fully backwards compatible and avoid the need for a new config flag.

I'm thinking of something like

To back to [the root]($path_to_root/index.md)

The $path_to_root part would be a relative link, so it works even if site-url is not set. The syntax could also be {{ path_to_root }} if you let handlebars render it.

JesusPerez · 2023-05-18T10:48:32Z

site-url value in book.toml did not work for me as far as I tried with mdbook v0.4.29

chapter-item are not changed for {{toc}}
**<base href ** only works for 404.page
{{ path_to_root }} for index.hbs has relative dots in hrefs

I can not rely on base href to solve all links and references use cases, dots in paths does not always help.
SUMMARY.md has to use relative paths

To solve this I made some changes in:

Changes can be seen in this fork (updated to v0.4.31) :

master...JesusPerez:mdBook:master

Obviously to complete the absolute paths in the pages I use a variable {{baseurl}} as a prefix of the href and src links.
How to use it is described here: how to use absolute links in content

[Introduction]({{urlbase}}introduction.md)
![Image]({{urlbase}}image.jpg)

KFearsoff

The code looks good to me, and the tests run, but unfortunately I wasn't able to actually confirm on the test_book the repo has that this PR fixes the issue. Is there some issue introduced in the latest commits that breaks this?

calvinbrewer · 2023-12-07T18:21:56Z

The code looks good to me, and the tests run, but unfortunately I wasn't able to actually confirm on the test_book the repo has that this PR fixes the issue. Is there some issue introduced in the latest commits that breaks this?

Confirmed here as well. Not working here: https://github.com/calvinbrewer/mdBook/tree/add_absolute_links_support with the configuration added as reference

joaofreires · 2023-12-07T21:31:30Z

The code looks good to me, and the tests run, but unfortunately I wasn't able to actually confirm on the test_book the repo has that this PR fixes the issue. Is there some issue introduced in the latest commits that breaks this?

Confirmed here as well. Not working here: https://github.com/calvinbrewer/mdBook/tree/add_absolute_links_support with the configuration added as reference

Sorry If I'm saying something wrong, but I opened this PR more than a year ago.
I did not include any examples in the test_book. Could you please clarify what you mean by 'Not working here'? What specific aspect are you testing? The proposed solution involves adding the site-url as a root in absolute paths. Therefore, in your configuration example, it would be /docs/. If you create a Markdown link in your test_book, such as [Prefix](/prefix.md) (note it starts with /), it should correctly generate an HTML a element with the href attribute set to /docs/prefix.html.

calvinbrewer · 2023-12-08T15:07:51Z

Sorry If I'm saying something wrong, but I opened this PR more than a year ago. I did not include any examples in the test_book. Could you please clarify what you mean by 'Not working here'? What specific aspect are you testing? The proposed solution involves adding the site-url as a root in absolute paths. Therefore, in your configuration example, it would be /docs/. If you create a Markdown link in your test_book, such as [Prefix](/prefix.md) (note it starts with /), it should correctly generate an HTML a element with the href attribute set to /docs/prefix.html.

I've added an example in the test_book test_book/src/individual/link_hr.md that links to the prefix.md here. This does not seem to have the site-url added to the rendered a href.

In addition, from a usability perspective, I'd assume the left nav items to also include the site-url as a prefix. I may be mistaken on the intent of the original absolute link support. Ideally, the entire mdBook application would live under a main domain as a micro application e.g. https://example.com/docs

joaofreires · 2023-12-08T16:03:13Z

I've added an example in the test_book test_book/src/individual/link_hr.md that links to the prefix.md here. This does not seem to have the site-url added to the rendered a href.

I have successfully built your branch, and the HTML output from your example seems to be producing the expected results. In test_book/book/individual/link_hr.html, the following segment is generated for your example:

....
<a href="/docs/prefix.html">This</a> should link to prefix.md.
....

IMO, this outcome aligns with our assumptions. Could you please share your build results? Maybe I missed something.

In addition, from a usability perspective, I'd assume the left nav items to also include the site-url as a prefix. I may be mistaken on the intent of the original absolute link support. Ideally, the entire mdBook application would live under a main domain as a micro application e.g. https://example.com/docs

My intention with this particular changes was not to address all those cases. Regarding the navigation links, they are generated based on SUMMARY.md, which, as per the mdBook documentation, requires all links to be relative.

KFearsoff · 2023-12-10T10:41:42Z

Therefore, in your configuration example, it would be /docs/. If you create a Markdown link in your test_book, such as [Prefix](/prefix.md) (note it starts with /), it should correctly generate an HTML a element with the href attribute set to /docs/prefix.html.

Hmm. I've tried doing this by myself, and after you wrote a response I checked again with the exact inputs you've provided. Here's a patch that I'm got for testing:

diff --git a/test_book/book.toml b/test_book/book.toml
index a305007..91e8c9d 100644
--- a/test_book/book.toml
+++ b/test_book/book.toml
@@ -9,6 +9,8 @@ edition = "2018"
 
 [output.html]
 mathjax-support = true
+site-url = "/docs/"
+use-site-url-as-root = true
 
 [output.html.playground]
 editable = true
diff --git a/test_book/src/individual/README.md b/test_book/src/individual/README.md
index 67a1e9d..fceb065 100644
--- a/test_book/src/individual/README.md
+++ b/test_book/src/individual/README.md
@@ -2,6 +2,8 @@
 
 This contains following tags:
 
+[Prefix](/prefix.md)
+
 - Headings
 - Paragraphs
 - Line breaks

I then run cargo run -- serve --hostname 127.0.0.1. My expectation is that at http://127.0.0.1:3000/individual/index.html, I should see a link to http://127.0.0.1:3000/docs/prefix.html, but I'm seeing a link to http://127.0.0.1:3000/prefix.html. Am I missing something?

joaofreires · 2023-12-10T17:09:06Z

@KFearsoff to view the results, it is necessary to execute the command cargo run -- build. This is because the server mode overrides the site-url configuration to "/" for development purposes, I presume.

You can check it here:

mdBook/src/cmd/serve.rs

Line 67 in c1d622e

book.config.set("output.html.site-url", "/").unwrap();

KFearsoff · 2023-12-10T20:37:13Z

@KFearsoff to view the results, it is necessary to execute the command cargo run -- build. This is because the server mode overrides the site-url configuration to "/" for development purposes, I presume.

Oh. Ohhh. Yeah, I can confirm this works.

This is... pretty unfortunate, though. I think serve overriding the setting is extremely unintuitive and does more harm than good, especially with this PR. Do you mind seeing if it's possible to remove, and can we afford making this breaking change? Because I'd really hate it if we can't.

joaofreires · 2023-12-12T23:50:43Z

@KFearsoff, I believe this constitutes a reasonable usage. The serve mode is designed exclusively for development purposes and is tasked solely with hosting the book files. I agree that enhanced documentation would be beneficial in this context though.

Another aspect to consider carefully is the content of site-url. It's important to note that this is not limited to pathnames; it can include absolute URLs complete with hostnames and schemas. This flexibility could potentially lead to further complications when operating on a local server if we want to include the site-url as is.

It would be good to have more feedbacks on it before any change.

JesusPerez · 2024-01-09T01:06:07Z

Just in case if it helps I updated my mdBook with absolute links via site-url based in current mdBook master branch (v0.4.40 f480534)

I only fixed a couple of files, maybe it is not enough but it seems to work for me.

You can clone it, build and try following how-to-use-it notes with sitefix-book as example.

For instance I have tested absolute URL with hostnames and schemas and it works !
@joaofreires I use to keep absolute path in most cases and leave the rest for web service resolution (server and browsers) in some circunstances "<base href=" in index.hbs can also help.

I have to deal with multi-books services with auth, for me it is critical to support absolute links.

Let me know if I can help on this.

joaofreires force-pushed the add_absolute_links_support branch from 9374e52 to addf00d Compare May 7, 2022 19:45

joaofreires mentioned this pull request May 7, 2022

site-url not respected in links #1764

Open

joaofreires added 2 commits September 9, 2022 20:34

add absolute links support

c1a0af3

update docs

d9c5f10

joaofreires force-pushed the add_absolute_links_support branch from addf00d to d9c5f10 Compare September 9, 2022 23:35

ehuss reviewed Oct 14, 2022

View reviewed changes

ehuss added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Oct 14, 2022

joaofreires added 3 commits November 9, 2022 11:47

Merge branch 'master' into add_absolute_links_support

bcd63f9

requested changes

e2b0b05

add new tests

2336e6a

joaofreires commented Nov 13, 2022

View reviewed changes

joaofreires requested a review from ehuss January 20, 2023 13:13

ehuss mentioned this pull request Jan 28, 2023

adressing relative to root not possible #1999

Open

mgeisler reviewed Feb 8, 2023

View reviewed changes

ehuss mentioned this pull request Jul 6, 2023

Trying to include files but in other directories--is this possible? #2114

Closed

joelouthan mentioned this pull request Jul 6, 2023

Handlebar reference a la asciidoc joelouthan/Theologicus#41

Closed

2 tasks

KFearsoff suggested changes Dec 6, 2023

View reviewed changes

victor-vb mentioned this pull request Aug 3, 2024

Add predefined reference files for compilation #2422

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add absolute links support #1802

Add absolute links support #1802

joaofreires commented May 7, 2022 •

edited

Loading

watsom27 commented Sep 6, 2022

joaofreires commented Sep 6, 2022

watsom27 commented Sep 7, 2022

schungx commented Sep 8, 2022

watsom27 commented Sep 29, 2022

ehuss left a comment

ehuss Oct 14, 2022 •

edited

Loading

joaofreires Nov 13, 2022

ehuss Oct 14, 2022

ehuss Oct 14, 2022

ehuss Oct 14, 2022

ehuss Oct 14, 2022

joaofreires Nov 13, 2022

mgeisler Feb 8, 2023

joaofreires Nov 13, 2022

mgeisler Feb 8, 2023

mgeisler commented Feb 8, 2023

JesusPerez commented May 18, 2023 •

edited

Loading

KFearsoff left a comment

calvinbrewer commented Dec 7, 2023

joaofreires commented Dec 7, 2023

calvinbrewer commented Dec 8, 2023 •

edited

Loading

joaofreires commented Dec 8, 2023

KFearsoff commented Dec 10, 2023

joaofreires commented Dec 10, 2023

KFearsoff commented Dec 10, 2023

joaofreires commented Dec 12, 2023

JesusPerez commented Jan 9, 2024 •

edited

Loading

		/// Prepend the `site_url` in links with absolute path.
		pub use_site_url_as_root: bool,

Add absolute links support #1802

Are you sure you want to change the base?

Add absolute links support #1802

Conversation

joaofreires commented May 7, 2022 • edited Loading

Prepend the site-url in links with absolute path.

Problem:

Proposal

TODO:

watsom27 commented Sep 6, 2022

joaofreires commented Sep 6, 2022

watsom27 commented Sep 7, 2022

schungx commented Sep 8, 2022

watsom27 commented Sep 29, 2022

ehuss left a comment

Choose a reason for hiding this comment

ehuss Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgeisler commented Feb 8, 2023

JesusPerez commented May 18, 2023 • edited Loading

KFearsoff left a comment

Choose a reason for hiding this comment

calvinbrewer commented Dec 7, 2023

joaofreires commented Dec 7, 2023

calvinbrewer commented Dec 8, 2023 • edited Loading

joaofreires commented Dec 8, 2023

KFearsoff commented Dec 10, 2023

joaofreires commented Dec 10, 2023

KFearsoff commented Dec 10, 2023

joaofreires commented Dec 12, 2023

JesusPerez commented Jan 9, 2024 • edited Loading

joaofreires commented May 7, 2022 •

edited

Loading

Prepend the `site-url` in links with absolute path.

ehuss Oct 14, 2022 •

edited

Loading

JesusPerez commented May 18, 2023 •

edited

Loading

calvinbrewer commented Dec 8, 2023 •

edited

Loading

JesusPerez commented Jan 9, 2024 •

edited

Loading